-
-
Notifications
You must be signed in to change notification settings - Fork 841
Document auto-closing keywords for PRs. #855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple textual suggestions
Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
I applied the suggested changes, and we now have 3 options:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long delay—missed this somehow.
The text is LGTM, for what it's worth. It might be helpful to have additional clarity on when they should/shouldn't be used, but that's above my pay grade, as is the overall decision on this PR, of course.
getting-started/git-boot-camp.rst
Outdated
to automatically close the linked issue once the PR is merged. However, you | ||
should avoid them if other PRs need to be merged before the issue can be | ||
closed (e.g. backports to other branches). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a new contributor, this might raise more questions than it answers. Can we link to a reference somewhere else in the devguide that tells contributors which PRs are generally backported, and which are not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To note, there's also other benefits to linking a PR beyond just auto-close—most notably, it adds an easy to find reference to the PR from the issue and the issue from the PR a number of handy places in the GitHub UI, which make navigation a fair bit quicker than manually/semi-automatically trying to figure out the corresponding PR (or issue).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nearly all bug fixes, doc fixes, and IDLE changes are backported, and nearly all feature changes are not. But even feature issues may require multiple PRs, so the above advice is still incomplete. And there are PRs without an issue. By linking autoclose to linking of issue to PR, GH has made issue-PR linking worse than useless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, unfortunately the story on GitHub around relationships of multiple PRs for one issue has never been good, and unlike a number of other pain points that have improved substantially in recent years, this one doesn't seem to have. And unlike UI issues, there's not as much Refined GitHub and tools like it can do about it—it is a big reason that GitHub overlays like ZenHub gained such a following
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately the story on GitHub around relationships of multiple PRs for one issue has never been good
I created this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey folks, I've captured this feedback! I hope that [tasklists] can help make this experienc better in the future. keep it coming :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @omerbensaadon ! We all really appreciate it!
I added a separate entry to the bootcamp about linking, including a rephrased version of the paragraph included in the previous commits. |
|
||
You can link to issues and pull requests using ``gh-NNNNN`` (this form is | ||
preferred over ``#NNNNN``). If the reference appears in a list, the link | ||
will be expanded to show the status and title of the issue/PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, is that really what triggers this? I've gotten inconsistent results in my resent testing, with links sometimes expanding and sometimes not in and outside of lists, even in the same spot. I'd be very curious to know.
Also, gh
(and #
?) references in commit messages get expanded to pythongh
—how does that happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, is that really what triggers this?
I think so, and it took me a while to figure that out too. ISTM that references like #855
are consistently expanded in lists like:
- Document auto-closing keywords for PRs. #855
- with some text Document auto-closing keywords for PRs. #855
- Document auto-closing keywords for PRs. #855
- with some text Document auto-closing keywords for PRs. #855
- Document auto-closing keywords for PRs. #855
- with some text Document auto-closing keywords for PRs. #855
But not in paragraphs like this (#855) or quotes like
Do you have any counterexamples?
Also, gh (and #?) references in commit messages get expanded to pythongh—how does that happen?
I've seen that and I think it's just a bug in GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with some text #855
Ah, I misinterpreted what you said originally to mean just bullets with no other text, when I was seeing them both with and without—silly me.
Do you have any counterexamples?
Not any others that I can reliably reproduce—I believe the behavior I saw (on the What's New issues) when I was trying to figure this out might have perhaps just been a combination of an issue where the link wasn't getting created as above when the linked issue hadn't been created when I loaded the page, and/or just not being totally sure what caused it when testing a number of things (particularly #NNN
vs org/repo#NNN
vs <full URL>
vs. ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One side note that came up during some related testing—AFAIK, GH-
/gh-
doesn't trigger the issue number lookup and autocompletion, which I find invaluable especially on repos nearing 6 digit issue and PR numbers like CPython.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #946 (review) the ref is not expanded, maybe because it's in a review?
Screenshot showing both the unexpanded list, and the popup on the first ref which confirms the ref is correct:
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Fixes now outdated #502 and supersedes #701.